Drop event dict keys colliding with LogRecord attributes in render_to_log_* - #842
Open
dualfroz wants to merge 1 commit into
Open
Drop event dict keys colliding with LogRecord attributes in render_to_log_*#842dualfroz wants to merge 1 commit into
dualfroz wants to merge 1 commit into
Conversation
dualfroz
force-pushed
the
dualfroz/fix-render-to-log-kwargs-reserved-keys
branch
from
September 5, 2026 23:10
c7f841c to
e8a3055
Compare
…_log_* render_to_log_kwargs() and render_to_log_args_and_kwargs() build the extra kwarg straight out of whatever is left in the event dict once the event, positional_args, and the exc_info/stack_info/stacklevel trio have been extracted. If a remaining key happens to share a name with an attribute that logging.LogRecord already carries (filename, module, process, message, ...), logging.Logger.makeRecord() rejects the whole call with a raw KeyError, crashing a documented, non-ProcessorFormatter configuration on an entirely plausible business field name. structlog.stdlib.ExtraAdder already filters LogRecord's own attribute names out of the event dict when merging a record's extra fields in the other direction. Apply the same filtering here so a colliding key is dropped from extra instead of reaching logging.Logger.makeRecord() and crashing it. Fixes hynek#486.
dualfroz
force-pushed
the
dualfroz/fix-render-to-log-kwargs-reserved-keys
branch
from
September 5, 2026 23:25
e8a3055 to
6c697cd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
structlog.stdlib.render_to_log_kwargs()andstructlog.stdlib.render_to_log_args_and_kwargs()are documented, standardprocessors for "rendering your log entries entirely within
logging" (seedocs/standard-library.md, "Processors" section). Both build theextrakwarg they hand to the standard library out of whatever event dict keys are
left after
event,positional_args, and theexc_info/stack_info/stackleveltrio have been extracted.If a remaining key happens to share a name with an attribute that
logging.LogRecordalready carries --filename,module,process,args,name,message, and so on are all plausible field names forsomeone's structured business data -- the standard library's own
Logger.makeRecord()rejects the call outright:No
ProcessorFormatterinvolved -- this is the plain "defer formatting tologging" configuration from the docs. The log call itself raises, takingdown whatever code path tried to log.
Root cause
src/structlog/stdlib.py,render_to_log_kwargs()(around line 1001,pre-fix) and
render_to_log_args_and_kwargs()(around line 961, pre-fix):after popping
event/positional_args/LOG_KWARG_NAMES, both functionsput the entire remaining event dict under
extraunconditionally. Theynever check it against the names
logging.LogRecordalready reserves foritself.
This is exactly the reverse of what
structlog.stdlib.ExtraAdderalreadydoes a few dozen lines above in the same file:
ExtraAddermerges aLogRecord's own extra fields into an event dict, and it explicitlyfilters out
_LOG_RECORD_KEYS(aLogRecord's standard attribute names) sothat record-internal fields do not leak in as noise.
render_to_log_kwargs/render_to_log_args_and_kwargsdo the same round trip in the oppositedirection (event dict ->
extraon a soon-to-be-builtLogRecord) but hadno equivalent filter.
Two names collide that are not in a fresh
LogRecord.__dict__at all:"message"and"asctime".Logger.makeRecord()special-cases both andrejects them regardless, because
Formatter.format()adds them later:Issue and prior state
This is #486, open since the
report against a Gunicorn integration. The maintainer identified the same
root cause in the issue thread and proposed the same fix direction ("this
needs to be fixed in
render_to_log_kwargsandrender_to_log_args_and_kwargsby not putting these fields under thesenames into
extra"), but the only change that shipped(commit
5400612, "docs/stdlib: add warning about ProcessorFormatter")only documents a different, narrower footgun: not to combine
render_to_log_kwargs/render_to_log_args_and_kwargswithProcessorFormatter(a real but separate problem, sinceProcessorFormatteradds its own
extra={"_logger": ..., "_name": ...}on top). It does nottouch the underlying collision, which reproduces with a plain, non-
ProcessorFormatterconfiguration as shown above.Fix
Reuse the existing
_LOG_RECORD_KEYSconstant (already computed from afresh
LogRecordforExtraAdder), extended with"message"and"asctime"as_RESERVED_LOG_RECORD_KEYS. Bothrender_to_log_kwargs()and
render_to_log_args_and_kwargs()now delete any event dict key thatintersects with that set before handing the remainder to
loggingasextra. Non-colliding data is passed through exactly as before.This drops the colliding field's value rather than renaming or raising.
That mirrors
ExtraAdder's own established precedent for the reversedirection in this same file, and it means the fix trades a crash for a
silent, best-effort log entry instead of no log entry (and a raised
exception) at all -- see "Flags" below for the alternative the maintainer's
wording also allows.